Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: JavaScript translations #20

Merged
merged 9 commits into from
Sep 18, 2023
Merged

fix: JavaScript translations #20

merged 9 commits into from
Sep 18, 2023

Conversation

BryanttV
Copy link
Contributor

@BryanttV BryanttV commented Sep 14, 2023

Description

This PR adds all configurations to use JavaScript translations.

How To Test

  1. Install this branch in your environment.
  2. Change the platform language in (Account → Site Preferences → Site Language). Set it to "Spanish (Latin America)"
  3. Add a block in a unit from Studio.
  4. You should see all the text of the block in the selected language.

Using translation commands

  1. Run make extract_translations
  2. Add the translations in each text.po file
  3. Run make compile_translations

Screenshots

image
image
image

@BryanttV BryanttV force-pushed the bav/fix-js-translations branch from 4cd0dbe to 85f3226 Compare September 15, 2023 13:20
@BryanttV BryanttV marked this pull request as ready for review September 15, 2023 13:27
@mariajgrimaldi
Copy link
Contributor

can you explain a bit more why it was failing?

@BryanttV
Copy link
Contributor Author

Can you explain a bit more why it was failing?

It was failing because the manage.py file, it takes the settings defined in the mindmap/settings/test.py file, but this file did not have statici18n added to INSTALLED_APPS, and neither the necessary configuration:

INSTALLED_APPS = (
    "statici18n" #  This allows to use the command
)

# statici18n
# https://django-statici18n.readthedocs.io/en/latest/settings.html

LANGUAGES = [
    ('en', 'English - Source Language'),
    ('es_419', 'Spanish (Latin America)'),
    ('es_ES', 'Spanish (Spain)'),
]

STATICI18N_DOMAIN = 'text'
STATICI18N_NAMESPACE = 'MindMapI18N'
STATICI18N_PACKAGES = (
    'mindmap',
)
STATICI18N_ROOT = 'mindmap/public/js'
STATICI18N_OUTPUT_DIR = 'translations'

Finally, the compile_translations command was updated to generate the JS files with the translations correctly:

compile_translations: 
    ...
    python manage.py compilejsi18n --namespace MindMapI18N --output $(JS_TARGET)

@BryanttV
Copy link
Contributor Author

Hi @johnvente, the text.js files are automatically generated by the compile_translations command, so we would have to update those files every time the command is executed.

@johnvente
Copy link
Contributor

Hi @johnvente, the text.js files are automatically generated by the compile_translations command, so we would have to update those files every time the command is executed.

Hi @BryanttV in that case doesn't worth to update the files, it's ok for me

@@ -7,6 +7,22 @@ function MindMapXBlock(runtime, element, context) {
const removeGradeURL = runtime.handlerUrl(element, "remove_grade");
const maxPointsAllowed = context.max_points;

let gettext;
if ("MindMapI18N" in window) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if("MindMapI18N" in window ||  "gettext" in window) {
  gettext = window.MindMapI18N?.gettext || window.gettext;
}

if (typeof gettext == "undefined") {
    // No translations -- used by test environment
    gettext = (string) => string; 
  }

@BryanttV BryanttV force-pushed the bav/fix-js-translations branch from 203e262 to 4670b80 Compare September 15, 2023 22:27
@BryanttV BryanttV requested a review from johnvente September 18, 2023 15:02
Copy link
Contributor

@mariajgrimaldi mariajgrimaldi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Copy link
Contributor

@johnvente johnvente left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job, LGTM!

@BryanttV BryanttV force-pushed the bav/fix-js-translations branch from 5a53e14 to 517449b Compare September 18, 2023 16:59
@BryanttV BryanttV merged commit 6dffdee into main Sep 18, 2023
@BryanttV BryanttV deleted the bav/fix-js-translations branch September 18, 2023 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants